Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Add KAFKA mocking #13

Merged
merged 10 commits into from
Jan 10, 2025
Merged

feat: Add KAFKA mocking #13

merged 10 commits into from
Jan 10, 2025

Conversation

SebastienDegodez
Copy link
Member

@SebastienDegodez SebastienDegodez commented Dec 5, 2024 •
edited by lbroudoux
Loading

Pull Request

Proposed Changes

The goal of this enhancement is to add KAFKA support to the testcontainers module.

For the moment, AsynchronousAPI mocking are not supported. This feature would require an additionnal container microcks-async-minion.

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request
  • run dotnet test and ensure you have test coverage for the lines you are introducing
  • run dotnet husky run and fix any issues that you have introduced

Reviewer

  • Label as either feature, fix, documentation, enhancement, maintenance or breaking

@SebastienDegodez
Copy link
Member Author

In JAVA, @lbroudoux introduce a notion MicrocksContainersEnsemble.
An ensemble allows you to configure and controle the multiple containers with one line.
It's a good choice to increase a developper experience.

But this approach use testcontainer lifecycle Startable, i don't see the similar usage in .NET.

@lbroudoux
Copy link
Member

In JAVA, @lbroudoux introduce a notion MicrocksContainersEnsemble. An ensemble allows you to configure and controle the multiple containers with one line. It's a good choice to increase a developper experience.

But this approach use testcontainer lifecycle Startable, i don't see the similar usage in .NET.

Yes, this approach was quite convenient in Java (and necessary to integrate with frameworks like Spring that can manage the testcontainers lifecycle). However it is typically not available in other Testcontainers bindings like Node or Go.

I think it's nice to keep this notion of MicrocksContainersEnsemble being different from a single MicrocksContainer and allowing reuse of this one + wrapping the other needed container. Also, I think it's important to stick with the idioms of each language/platform and thus the MicrocksContainersEnsemble would have its own ContainerBuilder while implementing DockerContainer even if - behind the scenes - it manages multiple containers. What do you think?

@SebastienDegodez
Copy link
Member Author

SebastienDegodez commented Dec 12, 2024 •
edited
Loading

Issue KafkaContainer (.NET) testcontainers/testcontainers-dotnet#1314

@SebastienDegodez
Copy link
Member Author

Contribution to add network on Kafka: testcontainers/testcontainers-dotnet#1316

@lbroudoux
Copy link
Member

Contribution to add network on Kafka: testcontainers/testcontainers-dotnet#1316

Hey @SebastienDegodez! I had the same issue with Kafka module on testcontainers Go unfortunately...

@SebastienDegodez
Copy link
Member Author

Contribution to add network on Kafka: testcontainers/testcontainers-dotnet#1316

Hey @SebastienDegodez! I had the same issue with Kafka module on testcontainers Go unfortunately...

Hey, I have develop the feature in the train this morning. For the moment, i replace the kafka_listener, protocole and advertised (in startupCallback).

@SebastienDegodez SebastienDegodez force-pushed the main branch 2 times, most recently from a14abb4 to e6a288c Compare December 19, 2024 00:12
SebastienDegodez and others added 6 commits December 19, 2024 01:17
@SebastienDegodez
ci: disable sonar for fork
4572834 
Signed-off-by: SebastienDegodez <[email protected]>
@SebastienDegodez
chore: ignore .idea
4c2568a 
Signed-off-by: SebastienDegodez <[email protected]>
@dependabot @SebastienDegodez
chore(deps): bump FluentAssertions from 6.12.0 to 7.0.0
5cfbab9 
Bumps [FluentAssertions](https://github.com/fluentassertions/fluentassertions) from 6.12.0 to 7.0.0.
- [Release notes](https://github.com/fluentassertions/fluentassertions/releases)
- [Changelog](https://github.com/fluentassertions/fluentassertions/blob/develop/AcceptApiChanges.ps1)
- [Commits](fluentassertions/fluentassertions@6.12.0...7.0.0)

---
updated-dependencies:
- dependency-name: FluentAssertions
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
@dependabot @SebastienDegodez
chore(deps): Bump xunit from 2.9.0 to 2.9.2
ab10a9c 
Bumps [xunit](https://github.com/xunit/xunit) from 2.9.0 to 2.9.2.
- [Commits](xunit/xunit@v2-2.9.0...v2-2.9.2)

---
updated-dependencies:
- dependency-name: xunit
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
@dependabot @SebastienDegodez
chore(deps): bump RestAssured.Net from 4.4.0 to 4.5.1
38e09ca 
Bumps [RestAssured.Net](https://github.com/basdijkstra/rest-assured-net) from 4.4.0 to 4.5.1.
- [Changelog](https://github.com/basdijkstra/rest-assured-net/blob/main/CHANGELOG.md)
- [Commits](basdijkstra/rest-assured-net@rest-assured-net-4.4.0...rest-assured-net-4.5.1)

---
updated-dependencies:
- dependency-name: RestAssured.Net
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
@SebastienDegodez
chore: update RestAssured logging configuration in tests (obsolete)
fc39251 
Signed-off-by: SebastienDegodez <[email protected]>
@SebastienDegodez SebastienDegodez marked this pull request as ready for review December 19, 2024 00:24
@SebastienDegodez
Copy link
Member Author

Hello @lbroudoux , "sonar.token is invalid"

I don't understand. We have lost a secret ?

@lbroudoux
Copy link
Member

lbroudoux commented Dec 19, 2024 •
edited
Loading

Hello @lbroudoux , "sonar.token is invalid"

I don't understand. We have lost a secret ?

I've checked and SONAR_TOKEN is still declared in configuration.

It is used here: https://github.com/microcks/microcks-testcontainers-dotnet/blob/main/.github/workflows/steps.dotnet-build-test.yml#L67

I think the issue you get is because the build is happening on your own branch (your fork) and not on the Microcks one. In that case, the secret we defined is not available. Maybe we should add an additional condition to prevent trying to launch the Sonar analysis? Will check this.

@SebastienDegodez
Copy link
Member Author

Hello @lbroudoux , "sonar.token is invalid"

I don't understand. We have lost a secret ?

I've checked and SONAR_TOKEN is still declared in configuration.

It is used here: https://github.com/microcks/microcks-testcontainers-dotnet/blob/main/.github/workflows/steps.dotnet-build-test.yml#L67

I think the issue you get is because the build is happening on your own branch (your fork) and not on the Microcks one. In that case, the secret we defined is not available. Maybe we should add an additional condition to prevent trying to launch the Sonar analysis? Will check this.

I can have a condition but to follow Sonar for all pr, it's not practical. For the future, do you think it is possible to add me the right to create and push only the branch in feature/* without risk for other repo or the main branch (or another strategy) ?

@SebastienDegodez SebastienDegodez force-pushed the main branch 4 times, most recently from f714277 to 4ceb187 Compare December 21, 2024 01:34
@SebastienDegodez SebastienDegodez marked this pull request as draft December 21, 2024 01:36
@SebastienDegodez SebastienDegodez force-pushed the main branch 3 times, most recently from 87049af to 88c1bba Compare December 21, 2024 11:42
@SebastienDegodez SebastienDegodez force-pushed the main branch 5 times, most recently from b1e20d6 to 6239e9b Compare January 1, 2025 22:28
@SebastienDegodez SebastienDegodez mentioned this pull request Jan 1, 2025
Closed
@SebastienDegodez SebastienDegodez force-pushed the main branch 2 times, most recently from 08ed304 to cadc0fc Compare January 2, 2025 01:03
@lbroudoux
Copy link
Member

Hey there! Sorry but I have been away from the keyboard due to vacations 😉
I'm now back to work and ready to start reviewing this on tomorrow!

@lbroudoux lbroudoux added this to the 0.2.0 milestone Jan 6, 2025
@lbroudoux lbroudoux added component/documentation Improvements or additions to documentation component/runtime Runtime behavior of test container component/settings Relates to API/settings availables kind/feature New feature labels Jan 6, 2025
@SebastienDegodez
Copy link
Member Author

Hey there! Sorry but I have been away from the keyboard due to vacations 😉 I'm now back to work and ready to start reviewing this on tomorrow!

Hello, Great !
I hope you are well rested.

I took the opportunity to fix/improve some commits.

lbroudoux
lbroudoux requested changes Jan 7, 2025
View reviewed changes
Copy link
Member

@lbroudoux lbroudoux left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is looks very good as usual. I made a few comments and change requests but it's more for my own understanding of for consistency concerns. Great job! 👏

Do you think we'd be ready to merge after those small changes?

lbroudoux
lbroudoux approved these changes Jan 10, 2025
View reviewed changes
Copy link
Member

@lbroudoux lbroudoux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks all good to me! I still have a few questions but they can be solved later on.

@lbroudoux lbroudoux merged commit 109d930 into microcks:main Jan 10, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lbroudoux lbroudoux lbroudoux approved these changes

Assignees
No one assigned
Labels
component/documentation Improvements or additions to documentation component/runtime Runtime behavior of test container component/settings Relates to API/settings availables kind/feature New feature
Projects
None yet
Milestone
0.2.0
Development

Successfully merging this pull request may close these issues.
2 participants
@SebastienDegodez @lbroudoux